Skip to content

Conversation

@shiltian
Copy link
Contributor

@shiltian shiltian commented Sep 14, 2024

No description provided.

Copy link
Contributor Author

shiltian commented Sep 14, 2024

@shiltian shiltian marked this pull request as ready for review September 14, 2024 17:26
@llvmbot
Copy link
Member

llvmbot commented Sep 14, 2024

@llvm/pr-subscribers-backend-amdgpu

@llvm/pr-subscribers-llvm-transforms

Author: Shilei Tian (shiltian)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/108713.diff

6 Files Affected:

  • (modified) llvm/include/llvm/Transforms/IPO/Attributor.h (+8-3)
  • (modified) llvm/lib/Transforms/IPO/Attributor.cpp (+28)
  • (modified) llvm/lib/Transforms/IPO/AttributorAttributes.cpp (+20-7)
  • (modified) llvm/test/Transforms/Attributor/address_space_info.ll (+1-1)
  • (modified) llvm/test/Transforms/Attributor/nocapture-1.ll (+2-2)
  • (modified) llvm/test/Transforms/Attributor/value-simplify.ll (+1-2)
diff --git a/llvm/include/llvm/Transforms/IPO/Attributor.h b/llvm/include/llvm/Transforms/IPO/Attributor.h
index 921fe945539510..07ac7c15c692ec 100644
--- a/llvm/include/llvm/Transforms/IPO/Attributor.h
+++ b/llvm/include/llvm/Transforms/IPO/Attributor.h
@@ -1332,7 +1332,7 @@ struct InformationCache {
   bool stackIsAccessibleByOtherThreads() { return !targetIsGPU(); }
 
   /// Return true if the target is a GPU.
-  bool targetIsGPU() {
+  bool targetIsGPU() const {
     return TargetTriple.isAMDGPU() || TargetTriple.isNVPTX();
   }
 
@@ -1341,6 +1341,8 @@ struct InformationCache {
   const ArrayRef<Function *>
   getIndirectlyCallableFunctions(Attributor &A) const;
 
+  unsigned getFlatAddressSpace(const Function *F);
+
 private:
   struct FunctionInfo {
     ~FunctionInfo();
@@ -1383,6 +1385,9 @@ struct InformationCache {
   /// through the information cache interface *prior* to looking at them.
   void initializeInformationCache(const Function &F, FunctionInfo &FI);
 
+  /// Return the assumed flat address space.
+  unsigned getAssumedFlatAddressSpace() const;
+
   /// The datalayout used in the module.
   const DataLayout &DL;
 
@@ -6267,8 +6272,8 @@ struct AAAddressSpace : public StateWrapper<BooleanState, AbstractAttribute> {
     return (AA->getIdAddr() == &ID);
   }
 
-  // No address space which indicates the associated value is dead.
-  static const uint32_t NoAddressSpace = ~0U;
+  // Invalid address space which indicates the associated value is dead.
+  static const uint32_t InvalidAddressSpace = ~0U;
 
   /// Unique ID (due to the unique address)
   static const char ID;
diff --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp
index 56d1133b25549a..b4197fdaf0c3ae 100644
--- a/llvm/lib/Transforms/IPO/Attributor.cpp
+++ b/llvm/lib/Transforms/IPO/Attributor.cpp
@@ -26,6 +26,7 @@
 #include "llvm/Analysis/InlineCost.h"
 #include "llvm/Analysis/MemoryBuiltins.h"
 #include "llvm/Analysis/MustExecute.h"
+#include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/IR/AttributeMask.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/Constant.h"
@@ -179,6 +180,12 @@ static cl::opt<bool> CloseWorldAssumption(
     "attributor-assume-closed-world", cl::Hidden,
     cl::desc("Should a closed world be assumed, or not. Default if not set."));
 
+static cl::opt<unsigned> AssumedFlatAddressSpace(
+    "attributor-assume-flat-address-space", cl::Hidden,
+    cl::desc(
+        "The assumed flat address space. This is mainly for test purpose."),
+    cl::init(~0U));
+
 /// Logic operators for the change status enum class.
 ///
 ///{
@@ -3294,6 +3301,27 @@ InformationCache::getIndirectlyCallableFunctions(Attributor &A) const {
   return IndirectlyCallableFunctions;
 }
 
+unsigned InformationCache::getFlatAddressSpace(const Function *F) {
+  if (!F)
+    return getAssumedFlatAddressSpace();
+  auto *TTI = getAnalysisResultForFunction<TargetIRAnalysis>(*F);
+  if (!TTI)
+    return getAssumedFlatAddressSpace();
+  return TTI->getFlatAddressSpace();
+}
+
+unsigned InformationCache::getAssumedFlatAddressSpace() const {
+  if (targetIsGPU()) {
+    if (TargetTriple.isAMDGPU() || TargetTriple.isNVPTX()) {
+      // We use 0 here directly instead of enumeration such that we don't need
+      // to include the target headers.
+      return 0;
+    }
+    llvm_unreachable("unknown GPU target");
+  }
+  return AssumedFlatAddressSpace;
+}
+
 void Attributor::recordDependence(const AbstractAttribute &FromAA,
                                   const AbstractAttribute &ToAA,
                                   DepClassTy DepClass) {
diff --git a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
index 217c7cccb5775a..9c775e48f28195 100644
--- a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
+++ b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
@@ -12571,8 +12571,20 @@ struct AAAddressSpaceImpl : public AAAddressSpace {
   void initialize(Attributor &A) override {
     assert(getAssociatedType()->isPtrOrPtrVectorTy() &&
            "Associated value is not a pointer");
-    if (getAssociatedType()->getPointerAddressSpace())
+
+    unsigned FlatAS =
+        A.getInfoCache().getFlatAddressSpace(getAssociatedFunction());
+    if (FlatAS == InvalidAddressSpace) {
+      indicatePessimisticFixpoint();
+      return;
+    }
+
+    unsigned AS = getAssociatedType()->getPointerAddressSpace();
+    if (AS != FlatAS) {
+      [[maybe_unused]] bool R = takeAddressSpace(AS);
+      assert(R && "The take should happen");
       indicateOptimisticFixpoint();
+    }
   }
 
   ChangeStatus updateImpl(Attributor &A) override {
@@ -12594,12 +12606,13 @@ struct AAAddressSpaceImpl : public AAAddressSpace {
 
   /// See AbstractAttribute::manifest(...).
   ChangeStatus manifest(Attributor &A) override {
-    Value *AssociatedValue = &getAssociatedValue();
-    Value *OriginalValue = peelAddrspacecast(AssociatedValue);
-    if (getAddressSpace() == NoAddressSpace ||
+    if (getAddressSpace() == InvalidAddressSpace ||
         getAddressSpace() == getAssociatedType()->getPointerAddressSpace())
       return ChangeStatus::UNCHANGED;
 
+    Value *AssociatedValue = &getAssociatedValue();
+    Value *OriginalValue = peelAddrspacecast(AssociatedValue);
+
     PointerType *NewPtrTy =
         PointerType::get(getAssociatedType()->getContext(), getAddressSpace());
     bool UseOriginalValue =
@@ -12646,17 +12659,17 @@ struct AAAddressSpaceImpl : public AAAddressSpace {
     if (!isValidState())
       return "addrspace(<invalid>)";
     return "addrspace(" +
-           (AssumedAddressSpace == NoAddressSpace
+           (AssumedAddressSpace == InvalidAddressSpace
                 ? "none"
                 : std::to_string(AssumedAddressSpace)) +
            ")";
   }
 
 private:
-  uint32_t AssumedAddressSpace = NoAddressSpace;
+  uint32_t AssumedAddressSpace = InvalidAddressSpace;
 
   bool takeAddressSpace(uint32_t AS) {
-    if (AssumedAddressSpace == NoAddressSpace) {
+    if (AssumedAddressSpace == InvalidAddressSpace) {
       AssumedAddressSpace = AS;
       return true;
     }
diff --git a/llvm/test/Transforms/Attributor/address_space_info.ll b/llvm/test/Transforms/Attributor/address_space_info.ll
index 73dd93c55b819b..d0e8cc67736ca7 100644
--- a/llvm/test/Transforms/Attributor/address_space_info.ll
+++ b/llvm/test/Transforms/Attributor/address_space_info.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --check-attributes --check-globals --prefix-filecheck-ir-name true
-; RUN: opt -aa-pipeline=basic-aa -passes=attributor -attributor-manifest-internal -attributor-annotate-decl-cs -S < %s | FileCheck %s --check-prefixes=CHECK
+; RUN: opt -aa-pipeline=basic-aa -passes=attributor -attributor-manifest-internal -attributor-annotate-decl-cs -attributor-assume-flat-address-space=0 -S < %s | FileCheck %s --check-prefixes=CHECK
 
 @dst = dso_local addrspace(1) externally_initialized global i32 0, align 4
 @g1 = dso_local addrspace(1) externally_initialized global ptr null, align 4
diff --git a/llvm/test/Transforms/Attributor/nocapture-1.ll b/llvm/test/Transforms/Attributor/nocapture-1.ll
index 3401ddfdd7d758..de5f31e470edfc 100644
--- a/llvm/test/Transforms/Attributor/nocapture-1.ll
+++ b/llvm/test/Transforms/Attributor/nocapture-1.ll
@@ -257,7 +257,7 @@ define i32 @nc1_addrspace(ptr %q, ptr addrspace(1) %p, i1 %b) {
 ; TUNIT-NEXT:    [[TMP:%.*]] = addrspacecast ptr addrspace(1) [[P]] to ptr
 ; TUNIT-NEXT:    [[TMP2:%.*]] = select i1 [[B]], ptr [[TMP]], ptr [[Q]]
 ; TUNIT-NEXT:    [[VAL:%.*]] = load i32, ptr [[TMP2]], align 4
-; TUNIT-NEXT:    store i32 0, ptr addrspace(1) [[P]], align 4
+; TUNIT-NEXT:    store i32 0, ptr [[TMP]], align 4
 ; TUNIT-NEXT:    store ptr [[Q]], ptr @g, align 8
 ; TUNIT-NEXT:    ret i32 [[VAL]]
 ;
@@ -272,7 +272,7 @@ define i32 @nc1_addrspace(ptr %q, ptr addrspace(1) %p, i1 %b) {
 ; CGSCC-NEXT:    [[TMP:%.*]] = addrspacecast ptr addrspace(1) [[P]] to ptr
 ; CGSCC-NEXT:    [[TMP2:%.*]] = select i1 [[B]], ptr [[TMP]], ptr [[Q]]
 ; CGSCC-NEXT:    [[VAL:%.*]] = load i32, ptr [[TMP2]], align 4
-; CGSCC-NEXT:    store i32 0, ptr addrspace(1) [[P]], align 4
+; CGSCC-NEXT:    store i32 0, ptr [[TMP]], align 4
 ; CGSCC-NEXT:    store ptr [[Q]], ptr @g, align 8
 ; CGSCC-NEXT:    ret i32 [[VAL]]
 ;
diff --git a/llvm/test/Transforms/Attributor/value-simplify.ll b/llvm/test/Transforms/Attributor/value-simplify.ll
index 68f179c88116e4..a5789790cc92a1 100644
--- a/llvm/test/Transforms/Attributor/value-simplify.ll
+++ b/llvm/test/Transforms/Attributor/value-simplify.ll
@@ -838,8 +838,7 @@ define void @user() {
 ; TUNIT: Function Attrs: mustprogress nofree norecurse nosync nounwind willreturn memory(write)
 ; TUNIT-LABEL: define {{[^@]+}}@user
 ; TUNIT-SAME: () #[[ATTR5]] {
-; TUNIT-NEXT:    [[TMP1:%.*]] = addrspacecast ptr addrspacecast (ptr addrspace(3) @ConstAS3Ptr to ptr) to ptr addrspace(3)
-; TUNIT-NEXT:    store i32 0, ptr addrspace(3) [[TMP1]], align 4
+; TUNIT-NEXT:    store i32 0, ptr addrspacecast (ptr addrspace(3) @ConstAS3Ptr to ptr), align 4
 ; TUNIT-NEXT:    ret void
 ;
 ; CGSCC: Function Attrs: mustprogress nofree nosync nounwind willreturn memory(write)

Copy link
Contributor

@arsenm arsenm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I mentioned on the other review, this should really be a module level property from the TargetMachine or DataLayout

@shiltian shiltian force-pushed the users/shiltian/no-assume-flat-addrspace branch from 98f4627 to 30c83b8 Compare September 16, 2024 03:13
@shiltian shiltian changed the base branch from main to users/shiltian/remove-get-flat-as-from-tti September 16, 2024 03:13
@shiltian shiltian force-pushed the users/shiltian/remove-get-flat-as-from-tti branch from 8db0d52 to f2331fc Compare September 18, 2024 02:22
@shiltian shiltian force-pushed the users/shiltian/no-assume-flat-addrspace branch 2 times, most recently from 7da66b7 to 6356a15 Compare September 18, 2024 02:24
@shiltian shiltian force-pushed the users/shiltian/remove-get-flat-as-from-tti branch from f2331fc to ad6ff10 Compare September 18, 2024 14:28
@shiltian shiltian force-pushed the users/shiltian/no-assume-flat-addrspace branch from 6356a15 to 02b52be Compare September 18, 2024 14:28
@shiltian shiltian force-pushed the users/shiltian/remove-get-flat-as-from-tti branch from ad6ff10 to 3541dca Compare September 18, 2024 17:09
@shiltian shiltian force-pushed the users/shiltian/no-assume-flat-addrspace branch from 02b52be to cfce39f Compare September 18, 2024 17:09
@shiltian shiltian force-pushed the users/shiltian/remove-get-flat-as-from-tti branch from 3541dca to e6fb6d5 Compare September 18, 2024 18:49
@shiltian shiltian force-pushed the users/shiltian/no-assume-flat-addrspace branch from cfce39f to fb2ed73 Compare September 18, 2024 18:49
@llvmbot llvmbot added backend:AMDGPU clang:openmp OpenMP related changes to Clang labels Sep 18, 2024
@shiltian shiltian force-pushed the users/shiltian/no-assume-flat-addrspace branch from fb2ed73 to 8a04c03 Compare September 25, 2024 18:49
@shiltian shiltian changed the base branch from users/shiltian/remove-get-flat-as-from-tti to main September 25, 2024 18:49
@shiltian
Copy link
Contributor Author

I unstacked from #108786 to unblock this.

Copy link
Member

@jdoerfert jdoerfert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG

Copy link
Contributor

@vidsinghal vidsinghal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@shiltian shiltian merged commit 0b7a18b into main Sep 27, 2024
@shiltian shiltian deleted the users/shiltian/no-assume-flat-addrspace branch September 27, 2024 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants